Skip to content

server: fix failed to remove template/iso if upload from local fails#4626

Merged
yadvr merged 2 commits into
apache:4.14from
ustcweizhou:4.14-fix-delete-template-upload-error
Mar 6, 2021
Merged

server: fix failed to remove template/iso if upload from local fails#4626
yadvr merged 2 commits into
apache:4.14from
ustcweizhou:4.14-fix-delete-template-upload-error

Conversation

@ustcweizhou
Copy link
Copy Markdown
Contributor

@ustcweizhou ustcweizhou commented Jan 27, 2021

Description

This PR fixes the issue that failed to remove template/iso if upload from local fails

When upload template/iso/volume from local fails, the install_path will not be a full path of file so removing it will fail.

mysql> select install_path from template_store_ref;
+--------------------------------------------------------------------+
| install_path                                                       |
+--------------------------------------------------------------------+
| template/tmpl/1/3/805f4763-248e-40ec-b79a-b868cc480d0a.qcow2       |
| template/tmpl/1/4/c7e32c9e-5e72-3726-85cf-aa5ccd84118d.qcow2       |
| template/tmpl/2/201/bc4f4f08-138a-31b8-af1a-d4450eff7982.qcow2     |
| template/tmpl/2/202                                                |
| template/tmpl/2/203/203-2-d47f8cde-a2a8-31e7-a826-2628ad98a6c8.iso |
| template/tmpl/2/204                                                |
| template/tmpl/5/205                                                |
| template/tmpl/2/206                                                |
| template/tmpl/2/207                                                |
| template/tmpl/2/208                                                |
| template/tmpl/2/209                                                |
| template/tmpl/2/210                                                |
+--------------------------------------------------------------------+
12 rows in set (0.00 sec)

mysql> select install_path from volume_store_ref;
+---------------------------------------------------------+
| install_path                                            |
+---------------------------------------------------------+
| volumes/2/22                                            |
| volumes/2/19/f93face9-6521-4184-b89a-cb07f86bbae8.qcow2 |
| volumes/2/23                                            |
| volumes/2/24                                            |
+---------------------------------------------------------+
4 rows in set (0.00 sec)

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@ustcweizhou
server: fix failed to remove template/iso if upload from local fails
52cd459 
When upload template/iso/volume from local fails, the install_path will not be a full path of file so removing it will fail.

```
mysql> select install_path from template_store_ref;
+--------------------------------------------------------------------+
| install_path                                                       |
+--------------------------------------------------------------------+
| template/tmpl/1/3/805f4763-248e-40ec-b79a-b868cc480d0a.qcow2       |
| template/tmpl/1/4/c7e32c9e-5e72-3726-85cf-aa5ccd84118d.qcow2       |
| template/tmpl/2/201/bc4f4f08-138a-31b8-af1a-d4450eff7982.qcow2     |
| template/tmpl/2/202                                                |
| template/tmpl/2/203/203-2-d47f8cde-a2a8-31e7-a826-2628ad98a6c8.iso |
| template/tmpl/2/204                                                |
| template/tmpl/5/205                                                |
| template/tmpl/2/206                                                |
| template/tmpl/2/207                                                |
| template/tmpl/2/208                                                |
| template/tmpl/2/209                                                |
| template/tmpl/2/210                                                |
+--------------------------------------------------------------------+
12 rows in set (0.00 sec)

mysql> select install_path from volume_store_ref;
+---------------------------------------------------------+
| install_path                                            |
+---------------------------------------------------------+
| volumes/2/22                                            |
| volumes/2/19/f93face9-6521-4184-b89a-cb07f86bbae8.qcow2 |
| volumes/2/23                                            |
| volumes/2/24                                            |
+---------------------------------------------------------+
4 rows in set (0.00 sec)
```
@ustcweizhou
server: disallow removing template/iso in NotUpload and UploadInProgr…
6d6d522 
…ess state
@yadvr yadvr added this to the 4.14.1.0 milestone Jan 28, 2021
@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jan 28, 2021

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 28, 2021

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 28, 2021

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2612

@shwstppr
Copy link
Copy Markdown
Contributor

shwstppr commented Jan 29, 2021

@blueorangutan test centos7 vmware-67u3

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 29, 2021

@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 29, 2021

Trillian test result (tid-3453)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45950 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4626-t3453-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
Smoke tests completed. 81 look OK, 2 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_01_deploy_kubernetes_cluster Failure 3613.69 test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster Failure 3601.97 test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster Failure 0.04 test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster Failure 0.04 test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster Failure 0.04 test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster Failure 0.04 test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster Failure 0.04 test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster Failure 0.04 test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown Error 38.33 test_kubernetes_clusters.py
test_02_create_template_with_checksum_sha1 Error 5.11 test_templates.py
test_03_create_template_with_checksum_sha256 Error 5.11 test_templates.py
test_04_create_template_with_checksum_md5 Error 5.12 test_templates.py

@yadvr yadvr requested a review from shwstppr February 1, 2021 08:58
@yadvr
Copy link
Copy Markdown
Member

yadvr commented Feb 2, 2021

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Feb 2, 2021

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Feb 2, 2021

Trillian test result (tid-3472)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35325 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4626-t3472-kvm-centos7.zip
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

shwstppr
shwstppr reviewed Feb 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ustcweizhou Local upload failed with a message,
Failed to upload file due to system misconfiguration. Please contact admin.
Now with changes, I cannot delete the template with message,
The template is either getting uploaded or it may be initiated shortly, please wait for it to be completed
Is this desired? I think we can allow deleting when it is in NotUploaded state

@weizhouapache
Copy link
Copy Markdown
Member

weizhouapache commented Feb 5, 2021

NotUploaded

@shwstppr NotUploaded is the initialed state of volumes/template/iso when it is uploaded from local.
when upload starts, it will be transited to UploadInProgress
when upload failed, it will be transited to UploadError

I think deleting template/iso in NotUploaded state should not be allowed (same as volumes).
when upload fails (by default template/isos/volumes will be in UploadError state after 1 hour), we can remove them.

Pearl1594
Pearl1594 approved these changes Feb 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@Pearl1594 Pearl1594 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified behavior. LGTM

@yadvr yadvr requested a review from shwstppr February 8, 2021 08:17
@yadvr yadvr modified the milestones: 4.14.1.0, 4.14.2.0 Feb 8, 2021
@yadvr
Copy link
Copy Markdown
Member

yadvr commented Feb 26, 2021 •
edited
Loading

ping @ustcweizhou @shwstppr @Pearl1594 - should we merge this on 4.14 now, or change base/milestone to 4.15.1.0?

@weizhouapache weizhouapache reopened this Feb 26, 2021
@weizhouapache weizhouapache reopened this Feb 26, 2021
@yadvr yadvr merged commit 58a3f90 into apache:4.14 Mar 6, 2021
@weizhouapache weizhouapache mentioned this pull request Jul 22, 2021
Closed
nlgordon pushed a commit to ippathways/cloudstack that referenced this pull request Aug 2, 2022
@ustcweizhou @nlgordon
server: fix failed to remove template/iso if upload from local fails (a…
75a0a9d 
…pache#4626)

* server: fix failed to remove template/iso if upload from local fails

When upload template/iso/volume from local fails, the install_path will not be a full path of file so removing it will fail.

```
mysql> select install_path from template_store_ref;
+--------------------------------------------------------------------+
| install_path                                                       |
+--------------------------------------------------------------------+
| template/tmpl/1/3/805f4763-248e-40ec-b79a-b868cc480d0a.qcow2       |
| template/tmpl/1/4/c7e32c9e-5e72-3726-85cf-aa5ccd84118d.qcow2       |
| template/tmpl/2/201/bc4f4f08-138a-31b8-af1a-d4450eff7982.qcow2     |
| template/tmpl/2/202                                                |
| template/tmpl/2/203/203-2-d47f8cde-a2a8-31e7-a826-2628ad98a6c8.iso |
| template/tmpl/2/204                                                |
| template/tmpl/5/205                                                |
| template/tmpl/2/206                                                |
| template/tmpl/2/207                                                |
| template/tmpl/2/208                                                |
| template/tmpl/2/209                                                |
| template/tmpl/2/210                                                |
+--------------------------------------------------------------------+
12 rows in set (0.00 sec)

mysql> select install_path from volume_store_ref;
+---------------------------------------------------------+
| install_path                                            |
+---------------------------------------------------------+
| volumes/2/22                                            |
| volumes/2/19/f93face9-6521-4184-b89a-cb07f86bbae8.qcow2 |
| volumes/2/23                                            |
| volumes/2/24                                            |
+---------------------------------------------------------+
4 rows in set (0.00 sec)
```

* server: disallow removing template/iso in NotUpload and UploadInProgress state
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shwstppr shwstppr shwstppr approved these changes

@Pearl1594 Pearl1594 Pearl1594 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.14.2.0

Development

Successfully merging this pull request may close these issues.

6 participants

@ustcweizhou @yadvr @blueorangutan @shwstppr @weizhouapache @Pearl1594